Moving away from msdia managed wrapper, to doing P/Invoke on msdia.#1018
Moving away from msdia managed wrapper, to doing P/Invoke on msdia.#1018mayankbansal018 merged 9 commits intomicrosoft:masterfrom mayankbansal018:pinvokedia
Conversation
need to fix acquisition of msdia for portable, this PR will fail
|
@dotnet-bot test this please |
| catch (COMException) | ||
| { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Throw exception here and Dispose
scripts/build.ps1
Outdated
| $fullCLRPackageDir = Get-FullCLRPackageDirectory | ||
| $coreCLRPackageDir = Get-CoreCLRPackageDirectory | ||
| $coreCLR20PackageDir = Get-CoreCLR20PackageDirectory | ||
| $coreCLR20TestHostPackageDir = Get-CoreCLR20TestHostPackageDirectory |
| /// <summary> | ||
| /// Manifest files for Reg Free Com. This is essentially put in place for the msdia dependency. | ||
| /// </summary> | ||
| private const string ManifestFileNameX86 = "TestPlatform.ObjectModel.x86.manifest"; |
There was a problem hiding this comment.
Remove vstest/src/testhost.x86/TestPlatform.ObjectModel.*.manifest files.
| if (HResult.Failed(this.source.LoadDataForExe(filename, searchPath, IntPtr.Zero))) | ||
| { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
throw exception with message here.
| { | ||
| this.session.findLinesByAddr(symbol.addressSection, symbol.addressOffset, (uint)symbol.length, out lines); | ||
| // Get the address section | ||
| if (HResult.Failed(symbol.GetAddressSection(out uint section))) |
There was a problem hiding this comment.
Should we combined all three 'if' statements with ||.
There was a problem hiding this comment.
we can, but I think code will be less readable.
|
|
||
| public static IDiaDataSource GetDiaSourceObject() | ||
| { | ||
| var currentDirectory = new ProcessHelper().GetCurrentProcessLocation(); |
There was a problem hiding this comment.
Check how we are handling /Inproc scenario.
There was a problem hiding this comment.
Will check this once InProc comes into play. We will have to make some changes then definitely, will let faizan know
| IntPtr modHandle = IntPtr.Zero; | ||
| if (IntPtr.Size == 8) | ||
| { | ||
| modHandle = LoadLibraryEx(Path.Combine(currentDirectory, "x64\\msdia140.dll"), IntPtr.Zero, 0); |
There was a problem hiding this comment.
Should we Path.PathSeparator here?
| if(modHandle == IntPtr.Zero) | ||
| { | ||
| // Failed to load msdia140, why? | ||
| return null; |
There was a problem hiding this comment.
Log message or throw here.
| public partial class ProcessHelper : IProcessHelper | ||
| { | ||
| /// <inheritdoc/> | ||
| public string GetCurrentProcessLocation() |
There was a problem hiding this comment.
Method name won't match defination, Should we rename to EntryPointDirectory?
There was a problem hiding this comment.
from users perspective in netcore, the process would be the dll he/she is creating.
3 participants
###Test Matrix